Skip to content

fix(binder): preserve reservations during bind cache lag - #2049

Closed
davidLif wants to merge 1 commit into
mainfrom
codex/fix-reservation-bindrequest-cache-lag
Closed

fix(binder): preserve reservations during bind cache lag#2049
davidLif wants to merge 1 commit into
mainfrom
codex/fix-reservation-bindrequest-cache-lag

Conversation

@davidLif

Copy link
Copy Markdown
Collaborator

Description

Fixes reservation pod cleanup during BindRequest/pod cache lag.

When SyncForGpuGroup sees a reservation pod but does not see any fraction pods for the GPU group, it uses BindRequests as a guard before deleting the reservation. The previous guard ignored succeeded BindRequests. In practice, the BindRequest status can advance before the pod GPU-group label is visible through the client cache, so cleanup could delete the reservation pod in the middle of a successful bind.

This keeps succeeded BindRequests protective while their target pod is still live. Failed BindRequests and succeeded BindRequests whose pod is gone or terminal do not protect the reservation.

Related Issues

N/A

Checklist

Note: Ensure your PR title follows the Conventional Commits format (e.g., feat(scheduler): add new feature)

  • Self-reviewed
  • Added/updated tests (if needed)
  • Updated documentation (if needed)
  • Added a changelog fragment via make changelog (or applied the skip-changelog label). Do not edit CHANGELOG.md directly — pending fragments are folded into it at release time.

Breaking Changes

None.

Additional Notes

Validation run:

go test ./pkg/binder/binding/resourcereservation -count=1
KUBEBUILDER_ASSETS="$(pwd)/$(bin/setup-envtest use 1.34.0 -p path --bin-dir bin)" go test ./pkg/binder/controllers/integration_tests -run TestAPIs -count=1
git diff --check

Signed-off-by: davidLif <davidshani12@gmail.com>
@github-actions

Copy link
Copy Markdown

Total coverage: 54.5% -> 54.5% (delta 0.00%)

Merging this branch will decrease overall coverage

Impacted Packages Coverage Δ 🤖
github.com/kai-scheduler/KAI-scheduler/pkg/binder/binding/resourcereservation 91.41% (-1.39%) 👎

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/kai-scheduler/KAI-scheduler/pkg/binder/binding/resourcereservation/resource_reservation.go 91.41% (-1.39%) 256 (+20) 234 (+15) 22 (+5) 👎

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/kai-scheduler/KAI-scheduler/pkg/binder/binding/resourcereservation/resource_reservation_test.go

}

for _, gpuGroup := range bindRequest.Spec.SelectedGPUGroups {
if slices.Contains(resources.GetGpuGroups(pod), gpuGroup) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the effect of this loop? We return true afterwards anyway, right?


func (rsc *service) hasLivePodForBindRequest(ctx context.Context, bindRequest *schedulingv1alpha2.BindRequest) (bool, error) {
pod := &v1.Pod{}
err := rsc.kubeClient.Get(ctx, client.ObjectKey{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider also matching the UID of the pod (against the BindRequests's OwnerReference) since a pod can be recreated with the same name.

@davidLif davidLif closed this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants